Skip to content

fix(lint): validateChartBindings resolves a report's dataset and rows/columns whether or not it has a chart - #16397

Merged
baozhoutao merged 2 commits into
mainfrom
claude/issue-16105-chart-bindings-chartless-report-dimensions
Sep 6, 2026
Merged

fix(lint): validateChartBindings resolves a report's dataset and rows/columns whether or not it has a chart#16397
baozhoutao merged 2 commits into
mainfrom
claude/issue-16105-chart-bindings-chartless-report-dimensions

Conversation

@baozhoutao

Copy link
Copy Markdown
Contributor

Fixes #16105

What was wrong

validateChartBindings reached a report through one closure, checkReportChart, whose first line was if (!isRec(chart)) return;. That closure was the only site that ever received report.dataset, and it was called with dataset, values, xAxis, yAxis, ownSelection and series — never with the report's dimension selection. Two gaps followed, both reproduced here:

  1. A report authored without a chart was not checked at all. Its dataset was never resolved, so a binding to a dataset that does not exist published clean. Its values measures were invisible for the same reason.
  2. rows / columns were resolved on no report, charted or not. On one and the same charted report object the measure selection was resolved against the dataset and the dimension selection beside it was not.

Premise check, before the first edit

  • The card's unverified sub-claim holds. grep -n 'rows\|columns' packages/lint/src/validate-chart-bindings.ts on origin/main returns 9 lines — 12, 18, 31, 230, 278, 291, 305, 345 and 371 — every one of them prose in the module docblock or in a message string about ADR-0021 result rows. No code path forwards report.rows / report.columns. Control on the same file: values occurs 33 times, and those are code.
  • The second early return at :510 has no defect of the same shape, and here is why. checkListChart takes the dataset FROM the chart (chart.dataset), not from the container, so a container without a chart has no dataset binding to resolve. That is a fact about the schema, not a reading of the walk: in packages/spec/src/ui/view.zod.ts the string dataset occurs at exactly one declaration site, line 1275, inside ListChartConfigSchema, where it is REQUIRED. ListViewShapeSchema (line 1805, the container that carries chart: ListChartConfigSchema.optional() at 1923) declares no dataset of its own; a grep for dataset over lines 1600 to 2050 returns zero. The dataset-bound page-component surface is the same story — properties.dataset is what marks a component checkable at all. So the :510 return skips a container that binds nothing, and no lift applies to it. Not touched.
  • The spec settles the ambiguity the brief flagged. ReportSchema declares rows as "Dimension names (from the dataset) to group rows by (down axis)" and columns as "Dimension names across (ADR-0021 D2)", both z.array(z.string()); values is "Measure names (from the dataset) to display". checkReportOrder in the same file already treats the three exactly that way — an order key must name "a rows/columns dimension or a values measure". There is no schema-permitted derived or bucketed key at these positions, so chart-dimension-unknown is not being widened onto a shape the spec allows, and there is no ambiguity left to record.
  • No open PR touches validate-chart-bindings.*; git ls-remote --heads origin 'claude/issue-16105*' named only this branch.

The shape

Dataset resolution is lifted out of the chart closure into resolveDataset, called once per report and once per block BEFORE the chart question is asked. The resolved dataset is then handed to two groups of positions:

  • the report's own selection — rows and columns to chart-dimension-unknown, values to chart-measure-unknown;
  • when a chart is present, its axis refs, with semantics untouched (xAxis, yAxis, ownSelection, series; chart-axis-not-selected stays a warning and still resolves against the chart's own chart.yAxis).

One path entered unconditionally, with the chart as the branch it always was — not a second "chartless reports too" pass after the early return. That is also what keeps an unresolvable dataset ONE finding: resolveDataset runs once per report surface, not once per group. Blocks of a joined report carry the same keys and go through the same helper, one shape applied twice.

ChartBinding.dimensions becomes a LIST of selections, because a report has two and the list-view and page surfaces have one. Each entry keeps its own path, so a finding names reports[i].columns[j] rather than a position in a merged list nobody wrote.

No new rule id, no severity moved. Two message corrections ride along, both stated in the changeset:

  • the report dataset finding now points at reports[i].dataset, the key the author wrote — it used to say reports[i].chart.dataset, a position a report does not have and a chartless report cannot have;
  • its sentence ends "there is no data to render" rather than "the chart has no data to render".

Tests

Head sha for every reading below: 081938a3c.

Four pins mirroring the card's four injections, each reading the findings ARRAY (rule id, path, severity) rather than an exit code — chart-axis-not-selected is a warning and changes no exit code, so a pass/fail assertion could not tell the tiers apart. Two of the four are the card's working controls, pinned so a later refactor cannot break the charted path while the new chartless assertions stay green.

pnpm --filter @objectstack/lint exec vitest run --maxWorkers=2 src/validate-chart-bindings.test.ts — 45 passed (35 before this change, all still green).

  • P1 chartless report, dataset renamed: exactly one chart-dataset-unknown at error, path reports[0].dataset, hint offers the real name.
  • P2 rows on a charted report, columns on a chartless matrix, rows on a chartless report: chart-dimension-unknown at error, paths reports[0].rows[0] / reports[0].columns[0].
  • Plus a chartless report's values (the same entrance, one collection over) and a joined block's dataset + rows with no chart on the block.
  • N1 charted report, dataset renamed: still gates, and exactly ONE finding — no double report after the lift.
  • N2 charted report, values renamed: chart-measure-unknown at error plus chart-axis-not-selected at warning, in that order, with exactly one error in the array.
  • N3 a clean charted report and a clean chartless report both report nothing; so does a joined container, which binds no dataset of its own.

Fixture re-scope. it('ignores a report with no chart') asserted a zero that held only because chartless reports were invisible. Its fixture is in fact CLEAN — dataset resolves, status is a declared dimension, task_count a declared measure — so the zero survives for a real reason. The test is renamed to say what it now reads ("says nothing about a chartless report whose every binding resolves") with a comment recording the re-scope. No assertion was weakened and nothing was deleted.

Ablation (implementation committed first; mutation and restore both proven on disk by blob hash, restore leg proven by an empty git diff HEAD):

  • mutation leg: git checkout 0374bcba9 -- packages/lint/src/validate-chart-bindings.ts, keeping the new tests. On-disk anchors before/after: checkReportSurface 3 to 0, resolveDataset 4 to 0, checkReportChart 0 to 3. Disk blob equals the base blob, so the mutation landed.
  • result: 7 failed, 38 passed. Every P pin went red; N2 and both N3 pins stayed green, which is what makes them controls rather than restatements.
  • N1 also reddened, and only on its path assertion — expected 'reports[0].chart.dataset' to be 'reports[0].dataset'. Its control substance (one finding, chart-dataset-unknown, error) asserted BEFORE the path and held on the old shape too. Reported as measured rather than as a discrimination the lift produced.
  • restore leg: bytes back to the HEAD blob, git diff HEAD empty for the target. No rebuild was needed for either leg and none was claimed: the test imports the subject by relative path within its own package, so vitest resolves src, not a dependency's exports to dist; packages/lint/vitest.config.ts sets one unrelated key and no alias.

Consumers. @objectstack/lint was rebuilt (packages/lint/dist/index.js carries checkReportSurface) and every DIRECT dependent was run: @objectstack/example-showcase 28 files, @objectstack/metadata-protocol 166 files, plus @objectstack/mcp, @objectstack/platform-objects, @objectstack/cloud-connection — all green through the shared verify lock (VERDICT command-exit 0). @objectstack/cli unit tier: 181 files, 2453 passed, 6 expected fail (integration tier declared to CI). Full @objectstack/lint suite: 100 files, 3410 passed, 5 skipped. pnpm --filter @objectstack/lint typecheck: VERDICT command-exit 0.

The examples emit no new findings, and that zero has a control. pnpm --filter @objectstack/example-showcase --filter @objectstack/example-todo validate passes on both — every rows / columns / values name in both apps' reports is declared by the bound dataset. Positive control, injected into examples/app-showcase/src/ui/reports/index.ts and proven on disk, then restored by blob hash: renaming the matrix report's rows and columns makes objectstack validate exit 1 with

  • report "showcase_status_priority_matrix": "status_nope" is not a dimension declared by dataset "showcase_task_metrics".
      rule: chart-dimension-unknown  at reports[1].rows[0]
  • report "showcase_status_priority_matrix": "priority_nope" is not a dimension declared by dataset "showcase_task_metrics".
      rule: chart-dimension-unknown  at reports[1].columns[0]

That report declares no chart, so this is the card's Fact 2 measured end to end through os validate on a real app.

Gates

node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derived 54 families; all 54 were run and all exit 0. Reconciliation: 54 derived, 54 run, 0 NOT-MEASURED, 0 UNRUN. Re-derived after git fetch origin main moved origin/main to 4998efa71 — the family list is byte-identical, so no family was missed by the older tree.

Two needed a second run and neither was a finding:

  • pnpm check:dual-build-cjs-loads first exited 3, PREREQUISITE NOT MET (seven packages had no dist/). After pnpm build, exit 0 — 103 require entry points across 66 packages load, 619 emitted CJS files parse.
  • pnpm check:type-check-debt exited 124 under a 300s harness timeout, then 3 twice (PREREQUISITE NOT MET, tsc OOM). The gate prints the cause: it re-measures under the caller's NODE_OPTIONS, this machine's default is --max-old-space-size=2096 and the gate's own pinned CI-shaped ceiling is 6144. At 6144 it exits 0 — 5 ledger entries re-measured in 90.8s, 55 raw tsc errors, none above its recorded number.

Repo-wide pnpm lint (eslint . --no-inline-config) exits 0, so no narrowing is claimed or needed. Control-character self-scan over the three changed files: no match.

Scope

Three files, no new rule id, no doc transcript moved (pnpm check:docs-transcript-drift green). No out-of-scope findings were filed — nothing outside this card's defect class turned up.

Card 15734 (which set chart-axis-not-selected resolves against on the report surface) and card 15462 (the widget-side empty-selection gap) both presume the chart entrance is taken; neither is addressed here. hotcrm card 1621's test/analytics-integrity.test.ts assertion stays where it is — retiring it is a decision for that repo once this lands.


Generated by Claude Code

…/columns whether or not it has a chart

`validateChartBindings` reached a report through one closure whose first line
was `if (!isRec(chart)) return`, and that closure was the only site that ever
received `report.dataset`. A report authored without a chart was therefore not
checked at all, and `rows` / `columns` were never resolved against the dataset
on any report — on one and the same report object the measure selection was
validated and the dimension selection beside it was not.

Dataset resolution is lifted out of the chart closure into `resolveDataset`,
called once per report and once per block before the chart question is asked.
The resolved dataset is then fed to two groups of positions: the report's own
selection (`rows` / `columns` -> `chart-dimension-unknown`, `values` ->
`chart-measure-unknown`) and, when a chart is present, its axis refs exactly as
before. One path entered unconditionally, not a second pass after the early
return, so an unresolvable dataset is still exactly one finding.

No new rule id and no severity moved. The report dataset finding now points at
`reports[i].dataset` rather than the position `reports[i].chart.dataset`, which
a report does not have, and its sentence no longer names a chart the report may
not draw.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vbw3RPgdtqesx4azk9SbW8
@github-actions github-actions Bot added the size/m label Sep 6, 2026
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

3 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 5 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 4998efa71773154561c471075f4ef12566ecc455packageMentionDocs.

Which tree this was computed on

This run read content/docs from d79370c03cf15d1fb66f5165d58433a048ea3a01 — the merge of head 081938a3c2f079b517ec96b2ef5ac48a70a3f771 into base 4998efa71773154561c471075f4ef12566ecc455, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin d79370c03cf15d1fb66f5165d58433a048ea3a01 && git checkout d79370c03cf15d1fb66f5165d58433a048ea3a01
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 4998efa71773154561c471075f4ef12566ecc455 081938a3c2f079b517ec96b2ef5ac48a70a3f771 && git checkout -B drift-repro 4998efa71773154561c471075f4ef12566ecc455 && git merge --no-ff 081938a3c2f079b517ec96b2ef5ac48a70a3f771

node scripts/docs-audit/affected-docs.mjs --json 4998efa71773154561c471075f4ef12566ecc455

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants